Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OTA-R] Fix retries on Query Image failure due to invalid session #18765

Merged
merged 9 commits into from
May 26, 2022

Conversation

isiu-apple
Copy link
Contributor

@isiu-apple isiu-apple commented May 24, 2022

Problem

When tester starts OTA-P with one set of arguments, initiates QueryImage, then kills and restarts OTA-P with a different set of arguments, the first QueryImage will fail due to invalid CASE session but the second QueryImage succeeds. For easier testing, the first QueryImage should also succeed after OTA-P is restarted.

Example sequence:

  • Requestor does one successful QueryImage (CASE established)
  • Provider killed
  • Provider relaunched
  • Requestor tries QueryImage again and gets the timeout error. Requestor invalidates the CASE.
[1653079824317] [28414:956800] CHIP: [DMG] Time out! failed to receive invoke command response from Exchange: 39593i
[1653079824317] [28414:956800] CHIP: [SWU] Received QueryImage failure response: ../../../examples/ota-requestor-app/linux/third_party/connectedhomeip/src/app/CommandSender.cpp:211: CHIP Error 0x00000032: Timeout
[1653079824317] [28414:956800] CHIP: [SWU] CASE session may be invalid, tear down session
  • Requestor tries QueryImage again and gets Query already in progress error.
[1653079824318] [28414:956800] CHIP: [SWU] Query already in progress
  • Initiate another QueryImage and now it succeeds.

Change overview

  • In DefaultOTARequestor::RecordNewUpdateState(), update new state before handling state transition.
  • In DefaultOTARequestorDriver::HandleIdleStateEnter(), add QueryImage retry cap.

Testing

  • Requestor does one successful QueryImage (CASE established)
  • Provider killed
  • Provider relaunched
  • Requestor tries QueryImage again and gets the timeout error -> requestor invalidates the CASE -> requestor tries QueryImage again and this time succeeds

* Requestor does one successful QueryImage (CASE established)
* Provider killed
* Provider relaunched
* Requestor tries QueryImage again and gets the timeout error -> requestor invalidates the CASE -> requestor tries queryImage again and this time succeeds
- Remove debug messages
@pullapprove pullapprove bot requested a review from yunhanw-google May 24, 2022 17:21
@isiu-apple isiu-apple self-assigned this May 24, 2022
@isiu-apple isiu-apple marked this pull request as draft May 24, 2022 17:21
@isiu-apple isiu-apple added the ota label May 24, 2022
@isiu-apple isiu-apple changed the title Issue k invalid session [OTA-R] Fix retries on DefaultOTARequestor::OnQueryImageFailure() due to IdleStateReason::kInvalidSession May 24, 2022
@isiu-apple isiu-apple changed the title [OTA-R] Fix retries on DefaultOTARequestor::OnQueryImageFailure() due to IdleStateReason::kInvalidSession [OTA-R] Fix retries on Query Image failure due to invalid session May 24, 2022
@isiu-apple isiu-apple marked this pull request as ready for review May 24, 2022 17:43
@github-actions
Copy link

github-actions bot commented May 24, 2022

PR #18765: Size comparison from b4586be to a47b62b

Increases (11 builds for cc13x2_26x2, cyw30739, efr32, nrfconnect)
platform target config section b4586be a47b62b change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read only) 676471 676503 32 0.0
.text 581436 581468 32 0.0
lock-mtd LP_CC2652R7 (read only) 625879 625911 32 0.0
.text 530956 530988 32 0.0
pump-app LP_CC2652R7 (read only) 676331 676363 32 0.0
.text 587076 587108 32 0.0
pump-controller-app LP_CC2652R7 (read only) 654307 654323 16 0.0
.text 570280 570296 16 0.0
cyw30739 ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 571178 571218 40 0.0
.app_xip_area 466244 466284 40 0.0
efr32 lighting-app BRD4161A (read only) 916020 916068 48 0.0
.text 916012 916060 48 0.0
BRD4161A+rpc (read only) 950192 950256 64 0.0
.text 950184 950248 64 0.0
BRD4161A+rs911x (read only) 790612 790660 48 0.0
.text 790604 790652 48 0.0
lock-app BRD4161A+wf200 (read only) 947080 947112 32 0.0
.text 947072 947104 32 0.0
window-app BRD4161A (read only) 897212 897276 64 0.0
.text 897204 897268 64 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1184539 1184571 32 0.0
text 812448 812480 32 0.0
Decreases (3 builds for cc13x2_26x2)
platform target config section b4586be a47b62b change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read/write) 166376 166344 -32 -0.0
pump-app LP_CC2652R7 (read/write) 167940 167908 -32 -0.0
pump-controller-app LP_CC2652R7 (read/write) 189564 189548 -16 -0.0
Full report (27 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section b4586be a47b62b change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 669743 669743 0 0.0
(read/write) 182112 182112 0 0.0
.bss 74836 74836 0 0.0
.data 3404 3404 0 0.0
.rodata 100655 100655 0 0.0
.text 568864 568864 0 0.0
lock-ftd LP_CC2652R7 (read only) 676471 676503 32 0.0
(read/write) 166376 166344 -32 -0.0
.bss 72884 72884 0 0.0
.data 3236 3236 0 0.0
.rodata 94551 94551 0 0.0
.text 581436 581468 32 0.0
lock-mtd LP_CC2652R7 (read only) 625879 625911 32 0.0
(read/write) 145716 145716 0 0.0
.bss 68620 68620 0 0.0
.data 3236 3236 0 0.0
.rodata 94431 94431 0 0.0
.text 530956 530988 32 0.0
pump-app LP_CC2652R7 (read only) 676331 676363 32 0.0
(read/write) 167940 167908 -32 -0.0
.bss 73284 73284 0 0.0
.data 3272 3272 0 0.0
.rodata 88771 88771 0 0.0
.text 587076 587108 32 0.0
pump-controller-app LP_CC2652R7 (read only) 654307 654323 16 0.0
(read/write) 189564 189548 -16 -0.0
.bss 73140 73140 0 0.0
.data 3232 3232 0 0.0
.rodata 83547 83547 0 0.0
.text 570280 570296 16 0.0
shell LP_CC2652R7 (read only) 662750 662750 0 0.0
(read/write) 184664 184664 0 0.0
.bss 77196 77196 0 0.0
.data 3408 3408 0 0.0
.rodata 97622 97622 0 0.0
.text 564900 564900 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 624734 624734 0 0.0
.app_xip_area 528012 528012 0 0.0
.bss 79364 79364 0 0.0
.data 708 708 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 627402 627402 0 0.0
.app_xip_area 532152 532152 0 0.0
.bss 77924 77924 0 0.0
.data 672 672 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 571178 571218 40 0.0
.app_xip_area 466244 466284 40 0.0
.bss 87312 87312 0 0.0
.data 584 584 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 916020 916068 48 0.0
(read/write) 133452 133452 0 0.0
.bss 131392 131392 0 0.0
.data 2060 2060 0 0.0
.text 916012 916060 48 0.0
BRD4161A+rpc (read only) 950192 950256 64 0.0
(read/write) 150136 150136 0 0.0
.bss 147872 147872 0 0.0
.data 2264 2264 0 0.0
.text 950184 950248 64 0.0
BRD4161A+rs911x (read only) 790612 790660 48 0.0
(read/write) 129720 129720 0 0.0
.bss 127652 127652 0 0.0
.data 2068 2068 0 0.0
.text 790604 790652 48 0.0
lock-app BRD4161A+wf200 (read only) 947080 947112 32 0.0
(read/write) 124188 124188 0 0.0
.bss 122164 122164 0 0.0
.data 2024 2024 0 0.0
.text 947072 947104 32 0.0
window-app BRD4161A (read only) 897212 897276 64 0.0
(read/write) 133504 133504 0 0.0
.bss 131456 131456 0 0.0
.data 2048 2048 0 0.0
.text 897204 897268 64 0.0
esp32 all-clusters-app c3devkit (read only) 1003030 1003030 0 0.0
(read/write) 1479978 1479978 0 0.0
.dram0.bss 69392 69392 0 0.0
.dram0.data 14624 14624 0 0.0
.flash.rodata 210536 210536 0 0.0
.flash.text 1003030 1003030 0 0.0
.iram0.text 62954 62954 0 0.0
m5stack (read only) 1057815 1057815 0 0.0
(read/write) 481992 481992 0 0.0
.dram0.bss 74912 74912 0 0.0
.dram0.data 34200 34200 0 0.0
.flash.rodata 240884 240884 0 0.0
.flash.text 1052431 1052431 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 683256 683256 0 0.0
.bss 80432 80432 0 0.0
.data 2016 2016 0 0.0
.text 599104 599104 0 0.0
lock k32w061+release (read/write) 729268 729268 0 0.0
.bss 80856 80856 0 0.0
.data 1976 1976 0 0.0
.text 644732 644732 0 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9348524 9348524 0 0.0
(read/write) 663057 663057 0 0.0
.bss 42241 42241 0 0.0
.data 1192 1192 0 0.0
.data.rel.ro 600760 600760 0 0.0
.dynamic 560 560 0 0.0
.got 15024 15024 0 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 456748 456748 0 0.0
.text 7377332 7377332 0 0.0
thermostat-no-ble arm64 (read only) 2360916 2360916 0 0.0
(read/write) 177457 177457 0 0.0
.bss 88193 88193 0 0.0
.data 1520 1520 0 0.0
.data.rel.ro 79944 79944 0 0.0
.dynamic 560 560 0 0.0
.got 4768 4768 0 0.0
.init 24 24 0 0.0
.init_array 376 376 0 0.0
.rodata 147812 147812 0 0.0
.text 1984032 1984032 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2419208 2419208 0 0.0
.bss 202860 202860 0 0.0
.data 5872 5872 0 0.0
.text 1381852 1381852 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1184539 1184571 32 0.0
bss 139560 139560 0 0.0
rodata 153684 153684 0 0.0
text 812448 812480 32 0.0
p6 all-clusters-app default (read/write) 2541304 2541304 0 0.0
.bss 137360 137360 0 0.0
.data 2808 2808 0 0.0
.text 1499568 1499568 0 0.0
light-app default (read/write) 2424616 2424616 0 0.0
.bss 129696 129696 0 0.0
.data 2608 2608 0 0.0
.text 1382880 1382880 0 0.0
lock-app default (read/write) 2435176 2435176 0 0.0
.bss 129504 129504 0 0.0
.data 2568 2568 0 0.0
.text 1393440 1393440 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 782900 782900 0 0.0
bss 70828 70828 0 0.0
noinit 40416 40416 0 0.0
text 553508 553508 0 0.0
lighting-app tlsr9518adk80d (read/write) 802924 802924 0 0.0
bss 71080 71080 0 0.0
noinit 40416 40416 0 0.0
text 570242 570242 0 0.0

@github-actions
Copy link

github-actions bot commented May 24, 2022

PR #18765: Size comparison from b4586be to baeceb0

Increases above 0.2%:

platform target config section b4586be baeceb0 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 .data 3236 3264 28 0.9
lock-mtd LP_CC2652R7 .data 3236 3264 28 0.9
pump-app LP_CC2652R7 .data 3272 3300 28 0.9
pump-controller-app LP_CC2652R7 .data 3232 3260 28 0.9
cyw30739 ota-requestor-no-progress-logging cyw930739m2evb_01 .data 584 612 28 4.8
efr32 lighting-app BRD4161A .data 2060 2088 28 1.4
BRD4161A+rpc .data 2264 2292 28 1.2
BRD4161A+rs911x .data 2068 2096 28 1.4
lock-app BRD4161A+wf200 .data 2024 2052 28 1.4
window-app BRD4161A .data 2048 2076 28 1.4
Increases (12 builds for cc13x2_26x2, cyw30739, efr32, linux, nrfconnect)
platform target config section b4586be baeceb0 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read/write) 166376 166608 232 0.1
.data 3236 3264 28 0.9
lock-mtd LP_CC2652R7 (read/write) 145716 145976 260 0.2
.data 3236 3264 28 0.9
pump-app LP_CC2652R7 .data 3272 3300 28 0.9
pump-controller-app LP_CC2652R7 (read/write) 189564 189812 248 0.1
.data 3232 3260 28 0.9
cyw30739 ota-requestor-no-progress-logging cyw930739m2evb_01 .data 584 612 28 4.8
efr32 lighting-app BRD4161A .data 2060 2088 28 1.4
BRD4161A+rpc .data 2264 2292 28 1.2
BRD4161A+rs911x .data 2068 2096 28 1.4
lock-app BRD4161A+wf200 (read/write) 124188 124196 8 0.0
.data 2024 2052 28 1.4
window-app BRD4161A .data 2048 2076 28 1.4
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9348524 9350020 1496 0.0
.text 7377332 7378772 1440 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1184539 1184571 32 0.0
Decreases (11 builds for cc13x2_26x2, cyw30739, efr32, nrfconnect)
platform target config section b4586be baeceb0 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 .bss 72884 72860 -24 -0.0
lock-mtd LP_CC2652R7 .bss 68620 68596 -24 -0.0
pump-app LP_CC2652R7 (read/write) 167940 167916 -24 -0.0
.bss 73284 73260 -24 -0.0
pump-controller-app LP_CC2652R7 (read only) 654307 654291 -16 -0.0
.bss 73140 73116 -24 -0.0
.text 570280 570264 -16 -0.0
cyw30739 ota-requestor-no-progress-logging cyw930739m2evb_01 .bss 87312 87288 -24 -0.0
efr32 lighting-app BRD4161A (read/write) 133452 133448 -4 -0.0
.bss 131392 131360 -32 -0.0
BRD4161A+rpc .bss 147872 147840 -32 -0.0
BRD4161A+rs911x (read/write) 129720 129716 -4 -0.0
.bss 127652 127620 -32 -0.0
lock-app BRD4161A+wf200 (read only) 947080 947064 -16 -0.0
.bss 122164 122140 -24 -0.0
.text 947072 947056 -16 -0.0
window-app BRD4161A (read/write) 133504 133500 -4 -0.0
.bss 131456 131424 -32 -0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 bss 139560 139532 -28 -0.0
text 812448 812444 -4 -0.0
Full report (27 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section b4586be baeceb0 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 669743 669743 0 0.0
(read/write) 182112 182112 0 0.0
.bss 74836 74836 0 0.0
.data 3404 3404 0 0.0
.rodata 100655 100655 0 0.0
.text 568864 568864 0 0.0
lock-ftd LP_CC2652R7 (read only) 676471 676471 0 0.0
(read/write) 166376 166608 232 0.1
.bss 72884 72860 -24 -0.0
.data 3236 3264 28 0.9
.rodata 94551 94551 0 0.0
.text 581436 581436 0 0.0
lock-mtd LP_CC2652R7 (read only) 625879 625879 0 0.0
(read/write) 145716 145976 260 0.2
.bss 68620 68596 -24 -0.0
.data 3236 3264 28 0.9
.rodata 94431 94431 0 0.0
.text 530956 530956 0 0.0
pump-app LP_CC2652R7 (read only) 676331 676331 0 0.0
(read/write) 167940 167916 -24 -0.0
.bss 73284 73260 -24 -0.0
.data 3272 3300 28 0.9
.rodata 88771 88771 0 0.0
.text 587076 587076 0 0.0
pump-controller-app LP_CC2652R7 (read only) 654307 654291 -16 -0.0
(read/write) 189564 189812 248 0.1
.bss 73140 73116 -24 -0.0
.data 3232 3260 28 0.9
.rodata 83547 83547 0 0.0
.text 570280 570264 -16 -0.0
shell LP_CC2652R7 (read only) 662750 662750 0 0.0
(read/write) 184664 184664 0 0.0
.bss 77196 77196 0 0.0
.data 3408 3408 0 0.0
.rodata 97622 97622 0 0.0
.text 564900 564900 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 624734 624734 0 0.0
.app_xip_area 528012 528012 0 0.0
.bss 79364 79364 0 0.0
.data 708 708 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 627402 627402 0 0.0
.app_xip_area 532152 532152 0 0.0
.bss 77924 77924 0 0.0
.data 672 672 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 571178 571178 0 0.0
.app_xip_area 466244 466244 0 0.0
.bss 87312 87288 -24 -0.0
.data 584 612 28 4.8
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 916020 916020 0 0.0
(read/write) 133452 133448 -4 -0.0
.bss 131392 131360 -32 -0.0
.data 2060 2088 28 1.4
.text 916012 916012 0 0.0
BRD4161A+rpc (read only) 950192 950192 0 0.0
(read/write) 150136 150136 0 0.0
.bss 147872 147840 -32 -0.0
.data 2264 2292 28 1.2
.text 950184 950184 0 0.0
BRD4161A+rs911x (read only) 790612 790612 0 0.0
(read/write) 129720 129716 -4 -0.0
.bss 127652 127620 -32 -0.0
.data 2068 2096 28 1.4
.text 790604 790604 0 0.0
lock-app BRD4161A+wf200 (read only) 947080 947064 -16 -0.0
(read/write) 124188 124196 8 0.0
.bss 122164 122140 -24 -0.0
.data 2024 2052 28 1.4
.text 947072 947056 -16 -0.0
window-app BRD4161A (read only) 897212 897212 0 0.0
(read/write) 133504 133500 -4 -0.0
.bss 131456 131424 -32 -0.0
.data 2048 2076 28 1.4
.text 897204 897204 0 0.0
esp32 all-clusters-app c3devkit (read only) 1003030 1003030 0 0.0
(read/write) 1479978 1479978 0 0.0
.dram0.bss 69392 69392 0 0.0
.dram0.data 14624 14624 0 0.0
.flash.rodata 210536 210536 0 0.0
.flash.text 1003030 1003030 0 0.0
.iram0.text 62954 62954 0 0.0
m5stack (read only) 1057815 1057815 0 0.0
(read/write) 481992 481992 0 0.0
.dram0.bss 74912 74912 0 0.0
.dram0.data 34200 34200 0 0.0
.flash.rodata 240884 240884 0 0.0
.flash.text 1052431 1052431 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 683256 683256 0 0.0
.bss 80432 80432 0 0.0
.data 2016 2016 0 0.0
.text 599104 599104 0 0.0
lock k32w061+release (read/write) 729268 729268 0 0.0
.bss 80856 80856 0 0.0
.data 1976 1976 0 0.0
.text 644732 644732 0 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9348524 9350020 1496 0.0
(read/write) 663057 663057 0 0.0
.bss 42241 42241 0 0.0
.data 1192 1192 0 0.0
.data.rel.ro 600760 600760 0 0.0
.dynamic 560 560 0 0.0
.got 15024 15024 0 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 456748 456748 0 0.0
.text 7377332 7378772 1440 0.0
thermostat-no-ble arm64 (read only) 2360916 2360916 0 0.0
(read/write) 177457 177457 0 0.0
.bss 88193 88193 0 0.0
.data 1520 1520 0 0.0
.data.rel.ro 79944 79944 0 0.0
.dynamic 560 560 0 0.0
.got 4768 4768 0 0.0
.init 24 24 0 0.0
.init_array 376 376 0 0.0
.rodata 147812 147812 0 0.0
.text 1984032 1984032 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2419208 2419208 0 0.0
.bss 202860 202860 0 0.0
.data 5872 5872 0 0.0
.text 1381852 1381852 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1184539 1184571 32 0.0
bss 139560 139532 -28 -0.0
rodata 153684 153684 0 0.0
text 812448 812444 -4 -0.0
p6 all-clusters-app default (read/write) 2541304 2541304 0 0.0
.bss 137360 137360 0 0.0
.data 2808 2808 0 0.0
.text 1499568 1499568 0 0.0
light-app default (read/write) 2424616 2424616 0 0.0
.bss 129696 129696 0 0.0
.data 2608 2608 0 0.0
.text 1382880 1382880 0 0.0
lock-app default (read/write) 2435176 2435176 0 0.0
.bss 129504 129504 0 0.0
.data 2568 2568 0 0.0
.text 1393440 1393440 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 782900 782900 0 0.0
bss 70828 70828 0 0.0
noinit 40416 40416 0 0.0
text 553508 553508 0 0.0
lighting-app tlsr9518adk80d (read/write) 802924 802924 0 0.0
bss 71080 71080 0 0.0
noinit 40416 40416 0 0.0
text 570242 570242 0 0.0

- Set kMaxInvalidSessionRetries to 1
- Remove mInvalidSessionRetryCount and use mProviderRetryCount instead
- Consolidate the code for updating mCurrentUpdateState.
- Update comments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants